Conversation
unflores
left a comment
unflores
left a comment
There was a problem hiding this comment.
Il faut revoir les besoins. Dis-moi si tu veux en parler.
| if ($extract_service->checkThemesIsAlreadySaved($extracted_themes)) { | ||
| $io->info('All themes already exist in the database'); |
There was a problem hiding this comment.
je ne pense pas qu'il soit une bonne idée de faire ce check comme extract service va create et update les themes.
| ) { | ||
| } | ||
|
|
||
| public function extract(): array |
There was a problem hiding this comment.
Maintenant tu peux tester extract meme sans ton fichier originel: https://github.com/unflores/di_example/blob/main/src/test/ThemeReaderTest.php#L22-L27
| $themes = $ExtractServices->GetThemesFromExcelFile($excel_file); | ||
| $preparedThemes = $ExtractServices->PrepareThemesForDatabase($themes); | ||
| $savedThemesCount = $ExtractServices->SaveThemesOnDatabase($preparedThemes); | ||
| $read_themes = new ThemeReader($this->sheet); |
There was a problem hiding this comment.
Tu ne dois pas melanger les tests. Normalment tu testerait theme reader seul, extract service seul et tu auras un test d'integration pour les deux. Ce que j'avais en tete etait plutot: https://github.com/unflores/di_example/blob/main/src/test/ThemeReaderTest.php#L22-L32
Typiquement ce dont je parle s'appelle un test unitaire, parce qu'il y a un seul unit(ou module) de code testé. Brèf, tu pourrais le changer plutard si tu as envie mais pour l'instant on peut avancer je pense...
2 participants
Modify ExtractService to accept a $sheet of type Worksheet in its constructor.
Remove the $excel_file parameter from the GetThemesFromExcelFile method.
Verify that the make imports/import-themes command runs without errors.